Updating to Platform 2.0.0 #66
Conversation
| * compliance with the License. You may obtain a copy of the License at | ||
| * http://license.openmrs.org | ||
| * | ||
| * Software distributed under the License is distributed on an "AS IS" |
There was a problem hiding this comment.
This class used org.openmrs.module.web.extension, unavailable on 2.0.0. Also, this class is not used in the code. That's why I removed it.
Should I revert this change and use the legacy UI module to provide org.openmrs.module.web.extension?
There was a problem hiding this comment.
@dkayiwa I think using the legacy UI module is better, so I updated the PR.
There was a problem hiding this comment.
Without this class, do the module's admin links still display on the admin page? https://qa-refapp.openmrs.org/openmrs/admin/index.htm
There was a problem hiding this comment.
It doesn't, I updated the PR.
wikumChamith
force-pushed
the
platform-2.0
branch
from
e2f6831 to
ff44719
Compare
| @@ -211,7 +211,7 @@ protected Object task() { | |||
| return null; | |||
| } | |||
|
|
|||
| }.executeInManualFlushMode(); | |||
| }.executeInAutoFlushMode(); | |||
There was a problem hiding this comment.
Are you sure this does not have any runtime side effects?
There was a problem hiding this comment.
When we run this in AutoFlushMode we get the following error: https://gist.github.com/wikumChamith/1a1c66da05937e3883d0f7c922871218
This might be caused because we have added Transactional here: https://github.com/openmrs/openmrs-core/blob/889ccc1cf4d13b4e09ae9ff3ea6713cb685b3cef/api/src/main/java/org/openmrs/api/db/hibernate/HibernateContextDAO.java#L319-L322
I tested this and didn't notice any problems. I think there is a chance this could slightly affect the performance.
| <dependency> | ||
| <groupId>org.simpleframework</groupId> | ||
| <artifactId>simple-xml</artifactId> | ||
| <version>2.7.1</version> |
There was a problem hiding this comment.
I added this to the test scope
wikumChamith
force-pushed
the
platform-2.0
branch
from
ff44719 to
82f7e45
Compare
| @@ -104,8 +110,8 @@ public void prepareImportServer() throws Exception { | |||
| @Override | |||
| public void runOnImportServerAfterImport() throws Exception { | |||
| Concept c = Context.getConceptService().getConcept(newConceptId); | |||
|
|
|||
| Assert.assertNull("The concept was found with the same id, wha happened??", c); | |||
There was a problem hiding this comment.
Do you mind explaining why you changed this?
There was a problem hiding this comment.
In platform 2.0 Context.getConceptService().getConcept(newConceptId); returns the concept. That's why I had to change the test.
|
|
||
| @Override | ||
| public List<?> prepareExportServer() throws Exception { | ||
|
|
||
| Concept c = new Concept(); | ||
| c.addName(new ConceptName("Test", Locale.US)); | ||
|
|
||
| c.addDescription(new ConceptDescription("Description", Locale.ENGLISH)); | ||
| concept = c; |
There was a problem hiding this comment.
In platform 2.0, the concept must have a description otherwise it gives the following error when saving.
'Concept #null' failed to validate with reason: descriptions: Concept.description.atLeastOneRequired
No description provided.